Skip to content

chore(deps): bump openmls to close the GCE dict-immutability bypass#3872

Open
tylerhawkes wants to merge 1 commit into
mainfrom
tyler/openmls-gce-bump
Open

chore(deps): bump openmls to close the GCE dict-immutability bypass#3872
tylerhawkes wants to merge 1 commit into
mainfrom
tyler/openmls-gce-bump

Conversation

@tylerhawkes

@tylerhawkes tylerhawkes commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

What

Bumps the pinned openmls fork rev from e125674 to 2c8d8aec, which adds one
commit: enforce app_data_dictionary immutability for bare
GroupContextExtensions proposals
(xmtp/openmls tyler/gce-dict-immutability).

Why

The app-data pre-v1.11 review found a bypass in the openmls fork: on a migrated
group (whose required_capabilities include AppDataUpdate), a member could send
a commit whose only proposal is a GroupContextExtensions proposal that swaps
arbitrary app_data_dictionary entries. openmls'
validate_app_data_update_proposals_and_group_context skipped its
CannotUpdateDictionaryDirectly check whenever the commit carried no
AppDataUpdate proposals, so the swap validated and merged — letting any member
poison COMPONENT_REGISTRY (wedging the group), fraudulently raise the
MIN_SUPPORTED_PROTOCOL_VERSION floor, or corrupt metadata. This routed around
the super-admin gating that the app-data floor/registry validation relies on.

The fix lives entirely in openmls (pure MLS-draft §4.7-6 compliance) and is being
upstreamed in openmls/openmls#2125. This PR pulls it into the fork rev libxmtp
pins so v1.11 ships with the hole closed. When the fork rebases onto upstream
(see openmls-fork-minimization-plan.md), the upstreamed commit supersedes this
carry.

The fix is byte-identical on the fork and on upstream main; it changes only
validate_app_data_update_proposals_and_group_context and adds a regression test.
All existing openmls app_data_update_proposal_validation tests still pass (108
on the fork feature matrix), plus the new
test_group_context_update_dictionary_without_app_data_update.

Note for the fork owner

The pin currently points at the branch tyler/gce-dict-immutability. Before the
v1.11 cut, this commit should be merged into the canonical shipping lineage
(insipx/bincode-fixes) and the pin re-pointed there, so the release doesn't ride
a personal branch.

Note

Bump openmls to patch GCE dict-immutability bypass

Updates the pinned git revision for all openmls* dependencies in Cargo.toml and crates/xmtp-workspace-hack/Cargo.toml from e125674 to 2c8d8ae to pick up a security fix for a GCE dict-immutability bypass.

Macroscope summarized 3e87b77.

🤖 Generated with Claude Code

@tylerhawkes
tylerhawkes requested a review from a team as a code owner July 17, 2026 08:20
@claude

claude Bot commented Jul 17, 2026

Copy link
Copy Markdown

Claude finished @tylerhawkes's task —— View job


PR Review Complete ✓

Review Date: July 17, 2026 08:21 UTC


Summary

This is a critical security patch that correctly bumps the openmls fork dependency from e125674 to 2c8d8aec to close a dictionary immutability bypass vulnerability. The changes are minimal, focused, and appropriate.

Changes Reviewed

The PR updates the openmls git rev across:

  • Cargo.toml (5 openmls packages): Cargo.toml:88-98
  • crates/xmtp-workspace-hack/Cargo.toml (5 openmls packages × 2 sections): crates/xmtp-workspace-hack/Cargo.toml:65-69, 157-161
  • Cargo.lock (7 openmls packages): all refs updated to new commit

Review Findings

✅ Code Quality

  • Dependency bump is consistent across all openmls packages (openmls, openmls_basic_credential, openmls_libcrux_crypto, openmls_rust_crypto, openmls_traits, openmls_memory_storage, openmls_test)
  • No version number changes, only git rev update (appropriate for fork workflow)
  • hakari workspace-hack correctly regenerated with matching revs in both [dependencies] and [build-dependencies] sections

✅ Security

✅ Test Coverage

  • libxmtp has extensive app_data validation tests in crates/xmtp_mls/src/groups/tests/test_validate_app_data_update.rs:1
  • Per PR description, openmls includes regression test test_group_context_update_dictionary_without_app_data_update
  • All 108 existing openmls app_data_update_proposal_validation tests pass

⚠️ Process Note

  • Before v1.11 release: The tyler/gce-dict-immutability branch pin must be merged to the canonical insipx/bincode-fixes branch and the pin updated. The PR correctly notes this but there's no tracking issue/todo referenced to ensure this happens before the release cut.

Recommendation

Approve with action item: Create a pre-release checklist item to re-point the openmls pin from tyler/gce-dict-immutability to insipx/bincode-fixes after the commit is merged into the canonical branch. Otherwise, v1.11 would ship with a personal branch dependency.


@macroscopeapp

macroscopeapp Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

This is a mechanical dependency version bump updating openmls git revisions to pull in an upstream security fix. No code changes in this repository, and the author owns all modified Cargo.toml files.

You can customize Macroscope's approvability policy. Learn more.

@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.55%. Comparing base (089279c) to head (3e87b77).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3872      +/-   ##
==========================================
+ Coverage   85.48%   85.55%   +0.07%     
==========================================
  Files         415      415              
  Lines       66906    66927      +21     
==========================================
+ Hits        57193    57262      +69     
+ Misses       9713     9665      -48     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants